Conversation
There was a problem hiding this comment.
Pull request overview
Updates the NGWMN XML helper to make the response generation more robust by explicitly controlling SQL column ordering and normalizing XML text values.
Changes:
- Added
_as_text()to avoid serializingNoneas the literal string"None"in XML. - Replaced
SELECT *with explicit column lists for lithology and well construction queries. - Updated XML builders to use
_as_text()and to explicitly index thePointIDfield for some record types.
You can also share your feedback on Copilot code review. Take the survey.
| sql = ( | ||
| 'select "PointID", "CasingTop", "CasingBottom", "CasingDepthUnits", ' | ||
| '"ScreenTop", "ScreenBottom", "ScreenBottomUnit", "ScreenDescription", "CasingDescription" ' | ||
| 'from "NMA_view_NGWMN_WellConstruction" where "PointID"=:point_id' | ||
| ) |
There was a problem hiding this comment.
make_well_construction_response now selects ScreenBottomUnit and CasingDescription, but make_well_construction() never writes those fields into the XML (it only uses indices 1-5 and 7). This makes the SQL harder to reason about and wastes work; either emit corresponding XML elements (e.g., include a unit element and casing description) or drop the unused columns from the SELECT so the tuple layout matches what is actually consumed.
| def _as_text(v): | ||
| return "" if v is None else str(v) | ||
|
|
There was a problem hiding this comment.
These changes alter NGWMN XML serialization (explicit column ordering + _as_text(None) => empty string) but there are no tests exercising services/ngwmn_helper.py output. Adding a focused unit test that builds a representative row and asserts the generated XML (including None handling and correct PointID/column-to-element mapping) would help prevent regressions as the NGWMN view schemas evolve.
Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
Notes
Any special considerations, workarounds, or follow-up work to note?